Skip to content

Conversation

lambdageek
Copy link
Contributor

Adds Windows resources with the rust version information to rustc-main.exe and rustc_driver.dll

Invokes rc.exe directly, rather than using one of the crates from the ecosystem to avoid adding dependencies.

A new internal rustc_windows_rc crate has the common build script machinery for locating rc.exe and constructing the resource script

@rustbot
Copy link
Collaborator

rustbot commented Aug 29, 2025

r? @jieyouxu

rustbot has assigned @jieyouxu.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

@rustbot rustbot added A-LLVM Area: Code generation parts specific to LLVM. Both correctness bugs and optimization-related issues. S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Aug 29, 2025
@rustbot
Copy link
Collaborator

rustbot commented Aug 29, 2025

These commits modify the Cargo.lock file. Unintentional changes to Cargo.lock can be introduced when switching branches and rebasing PRs.

If this was unintentional then you should revert the changes before this PR is merged.
Otherwise, you can ignore this comment.

Some changes occurred in compiler/rustc_codegen_ssa

cc @WaffleLapkin

@lambdageek

This comment was marked as outdated.

@bjorn3
Copy link
Member

bjorn3 commented Aug 29, 2025

Why is the rustc version included in the product name? At most including the release channel would make sense to me (given that nightly genuinely behaves differently from stable by allowing unstable features), but the exaxt version is duplicated with the product version field.

@rust-log-analyzer

This comment has been minimized.

@lambdageek
Copy link
Contributor Author

Why is the rustc version included in the product name?

I don't have a good justification for the first iteration of this PR. Happy to update the names/descriptions to whatever makes the most sense

@rustbot rustbot added the T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) label Aug 29, 2025
@rustbot
Copy link
Collaborator

rustbot commented Aug 29, 2025

This PR was rebased onto a different master commit. Here's a range-diff highlighting what actually changed.

Rebasing is a normal part of keeping PRs up to date, so no action is needed—this note is just to help reviewers.

@lambdageek
Copy link
Contributor Author

Updated the Product Description to be just "Rust Compiler" or "Rust Compiler (channel)" for non-stable

@rust-log-analyzer

This comment has been minimized.

@jieyouxu
Copy link
Member

jieyouxu commented Aug 30, 2025

This needs someone who actually have some clues about windows resources to review... I'll ask about a reviewer

@rustbot

This comment has been minimized.

@jieyouxu jieyouxu added the O-windows Operating system: Windows label Aug 30, 2025
@jieyouxu
Copy link
Member

jieyouxu commented Aug 30, 2025

Adds Windows resources with the rust version information to rustc-main.exe and rustc_driver.dll

I'm not familiar with Windows resources. But can you say more on the motivation for this change?

EDIT: okay I found #t-compiler/windows > version resources on rustc.exe and rustc_driver.dll, but it's still not super obvious to me the motivation for the change.

@jieyouxu jieyouxu assigned jieyouxu and unassigned ChrisDenton Aug 30, 2025
@lambdageek
Copy link
Contributor Author

lambdageek commented Aug 30, 2025

In many ways this is a cosmetic change: as you can see in the screenshot in the comment above, Windows shows the version info in the file explorer when you right click on the .exe or .dll and look at the details

However this info is also used by some other tools on Windows such as debuggers or crash reporters when collecting diagnostic information.

For our internal builds of Rust at Microsoft having version info available would allow us to collect better automated crash reports from our users.
I could just add this only in our internal builds, but it seemed like it would be useful to upstream it.

@jieyouxu
Copy link
Member

Ok thanks for the clarification, that makes sense. I'll ask internally for another reviewer who has at least slightly more clues about this than I do.

@rustbot rustbot assigned wesleywiser and unassigned jieyouxu Sep 3, 2025
@rustbot
Copy link
Collaborator

rustbot commented Sep 3, 2025

wesleywiser is currently at their maximum review capacity.
They may take a while to respond.

@mati865
Copy link
Member

mati865 commented Sep 4, 2025

add the support for windows-gnu (I'm not sure if this one is going to work) and windows-gnullvm

@mati865 you would need to use the windres resource compiler from the GNU toolchain. the .rc file format is the same. If you specify the output format as -O coff I think you should be able to just pass the result to the gnu linker (ie, I don't think the stuff in rustc_driver and rustc_main build scripts would need to change much).

I know, it's just not a priority for me.
With LD there might be a clash with default-manifest.o that is provided by default. Unlike LLVM tools, LD still cannot properly handle multiple resource files.

Copy link
Member

@wesleywiser wesleywiser left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm can you squash your commits together and force push?

View changes since this review

Adds Windows resources with the rust version information to rustc-main.exe and rustc_driver.dll

Sets the product description to "Rust Compiler" or "Rust Compiler (channel)" for non-stable channels
@lambdageek
Copy link
Contributor Author

@wesleywiser done

@wesleywiser
Copy link
Member

@bors r+

@bors
Copy link
Collaborator

bors commented Sep 8, 2025

📌 Commit 095fa86 has been approved by wesleywiser

It is now in the queue for this repository.

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Sep 8, 2025
jhpratt added a commit to jhpratt/rust that referenced this pull request Sep 8, 2025
…esleywiser

compiler: Add Windows resources to rustc-main and rustc_driver

Adds Windows resources with the rust version information to rustc-main.exe and rustc_driver.dll

Invokes `rc.exe` directly, rather than using one of the crates from the ecosystem to avoid adding dependencies.

A new internal `rustc_windows_rc` crate has the common build script machinery for locating `rc.exe` and constructing the resource script
bors added a commit that referenced this pull request Sep 8, 2025
Rollup of 11 pull requests

Successful merges:

 - #145177 (std: move `thread` into `sys`)
 - #146018 (compiler: Add Windows resources to rustc-main and rustc_driver)
 - #146025 (compiler: Include span of too huge array with `-Cdebuginfo=2`)
 - #146184 (In the rustc_llvm build script, don't consider arm64* to be 32-bit)
 - #146195 (fix partial urlencoded link support)
 - #146300 (Implement `Sum` and `Product` for `f16` and `f128`.)
 - #146314 (mark `format_args_nl!` as `#[doc(hidden)]`)
 - #146324 (const-eval: disable pointer fragment support)
 - #146326 (simplify the declaration of the legacy integer modules (`std::u32` etc.))
 - #146339 (Update books)
 - #146343 (Weakly export `platform_version` symbols)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors
Copy link
Collaborator

bors commented Sep 9, 2025

⌛ Testing commit 095fa86 with merge fefce3c...

@bors
Copy link
Collaborator

bors commented Sep 9, 2025

☀️ Test successful - checks-actions
Approved by: wesleywiser
Pushing fefce3c to master...

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label Sep 9, 2025
@bors bors merged commit fefce3c into rust-lang:master Sep 9, 2025
11 checks passed
@rustbot rustbot added this to the 1.91.0 milestone Sep 9, 2025
Copy link
Contributor

github-actions bot commented Sep 9, 2025

What is this? This is an experimental post-merge analysis report that shows differences in test outcomes between the merged PR and its parent PR.

Comparing e9b6085 (parent) -> fefce3c (this PR)

Test differences

Show 4 test diffs

4 doctest diffs were found. These are ignored, as they are noisy.

Test dashboard

Run

cargo run --manifest-path src/ci/citool/Cargo.toml -- \
    test-dashboard fefce3cecd63cebf2d7c9aa3dd90a84379fcfa1a --output-dir test-dashboard

And then open test-dashboard/index.html in your browser to see an overview of all executed tests.

Job duration changes

  1. dist-apple-various: 3117.2s -> 5295.3s (69.9%)
  2. dist-x86_64-apple: 5617.0s -> 9013.2s (60.5%)
  3. pr-check-1: 1732.5s -> 1359.0s (-21.6%)
  4. dist-various-1: 3738.0s -> 4319.2s (15.6%)
  5. aarch64-gnu-llvm-19-1: 3894.5s -> 3340.1s (-14.2%)
  6. i686-gnu-2: 6412.3s -> 5517.5s (-14.0%)
  7. aarch64-gnu-llvm-19-2: 2591.1s -> 2245.5s (-13.3%)
  8. x86_64-gnu-llvm-19-1: 3626.7s -> 3232.3s (-10.9%)
  9. x86_64-gnu-miri: 4931.9s -> 4410.0s (-10.6%)
  10. i686-gnu-1: 8272.1s -> 7423.4s (-10.3%)
How to interpret the job duration changes?

Job durations can vary a lot, based on the actual runner instance
that executed the job, system noise, invalidated caches, etc. The table above is provided
mostly for t-infra members, for simpler debugging of potential CI slow-downs.

@rust-timer
Copy link
Collaborator

Finished benchmarking commit (fefce3c): comparison URL.

Overall result: ✅ improvements - no action needed

@rustbot label: -perf-regression

Instruction count

Our most reliable metric. Used to determine the overall result above. However, even this metric can be noisy.

mean range count
Regressions ❌
(primary)
- - 0
Regressions ❌
(secondary)
- - 0
Improvements ✅
(primary)
- - 0
Improvements ✅
(secondary)
-0.1% [-0.1%, -0.1%] 1
All ❌✅ (primary) - - 0

Max RSS (memory usage)

This benchmark run did not return any relevant results for this metric.

Cycles

Results (secondary 0.6%)

A less reliable metric. May be of interest, but not used to determine the overall result above.

mean range count
Regressions ❌
(primary)
- - 0
Regressions ❌
(secondary)
3.8% [3.8%, 3.8%] 1
Improvements ✅
(primary)
- - 0
Improvements ✅
(secondary)
-2.6% [-2.6%, -2.6%] 1
All ❌✅ (primary) - - 0

Binary size

This benchmark run did not return any relevant results for this metric.

Bootstrap: 466.867s -> 467.971s (0.24%)
Artifact size: 387.52 MiB -> 387.55 MiB (0.01%)

@nico
Copy link

nico commented Sep 11, 2025

Hardcoding rc.exe instead of allowing llvm-rc or some other res-writing crate prevents cross-building rustc-for-Windows on non-Windows hosts, no?

@ilovepi
Copy link
Contributor

ilovepi commented Sep 11, 2025

Hardcoding rc.exe instead of allowing llvm-rc or some other res-writing crate prevents cross-building rustc-for-Windows on non-Windows hosts, no?

Yeah this is also a problem for builds of Fuchsia's toolchains. Is there some way we can specify the path to the build directly? Maybe via the config.toml? Or an environment variable?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-LLVM Area: Code generation parts specific to LLVM. Both correctness bugs and optimization-related issues. merged-by-bors This PR was explicitly merged by bors. O-windows Operating system: Windows S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.